Skip to content
This repository has been archived by the owner on Apr 1, 2022. It is now read-only.

Adds support for git sources in Podfile.lock tactic #345

Merged
merged 2 commits into from
Aug 27, 2021

Conversation

meghfossa
Copy link
Contributor

@meghfossa meghfossa commented Aug 26, 2021

Overview

Supports git sources in cocoapods. Intention is to reach parity between CLIv1, and CLIv2.

It also does minor refractor.

Acceptance criteria

  • Cocoapod sourced from git sources, result in git locator (as opposed to current behaviour of always using Pod)

Testing plan

  1. Create a Pod project (example Podfile provided below)
  2. Enforce git source for the pod (pod update)
  3. Run analysis, and inspect locators

Podfile

# Uncomment the next line to define a global platform for your project
# platform :ios, '9.0'

target 'someproj' do
  # Comment the next line if you don't want to use dynamic frameworks
  use_frameworks!

  # Pods for someproj
  pod 'Alamofire-test', :git => 'https://github.com/meghfossa/Alamo-fire.git'
  pod 'GoogleUtilities' 

  target 'someproj WatchKit AppTests' do
    inherit! :search_paths
    # Pods for testing
  end

  target 'someproj WatchKit AppUITests' do
    # Pods for testing
  end

  target 'someprojTests' do
    inherit! :search_paths
    # Pods for testing
  end

  target 'someprojUITests' do
    # Pods for testing
  end

end

Risks

Although, current documentation on cocoapod resolution is not ideal, I think potential risk is not resolving when subspecs are sourced from original git source. E.g ExampleKit is sourced from privateGit, and then ExampleKit/Logger should also be sourced from such.

Currently, this is also not supported in CLI v1. I intend to address this in subsequent PR.

References

Works on https://github.com/fossas/team-analysis/issues/721

Checklist

  • I added tests for this PR's change (or confirmed tests are not viable).
  • If this PR introduced a user-visible change, I added documentation into docs/.
  • I updated Changelog.md if this change is externally facing. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
  • I linked this PR to any referenced GitHub issues, if they exist.

@meghfossa meghfossa requested a review from zlav August 26, 2021 02:44
@meghfossa
Copy link
Contributor Author

Leaving change-log modification for post-approval/finalization.

Copy link
Member

@zlav zlav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, very simple. I don't have any real suggestions.


- Pods sourced from local path are not supported (e.g. `pod 'AFNetworking', :path => '~/Documents/AFNetworking'`).
- Pods sourced from http path are not supported (e.g `pod 'JSONKit', :podspec => 'https://example.com/JSONKit.podspec'`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would it take to support these? We have the url fetcher that we use with yarn analysis. These look more complex though, correct me if I'm wrong but we would have to download the podspec file during analysis and then parse that file to get the dependency information?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can still create the graph, but we would need to download the file to infer the licence informations. It also may require downloading of additional content based on the the podspec file for that purpose.

It is likely the least used source, I think we can ignore it for now, until there is an ask for it. It may involve a bit of work, on the core backend.

@meghfossa meghfossa merged commit 57664dd into master Aug 27, 2021
@meghfossa meghfossa deleted the feat/git-support-for-pods branch August 27, 2021 18:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants